Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

igraph dfs may signal unreachable nodes with 0 #518

Merged
merged 20 commits into from
Oct 14, 2024
Merged

Conversation

jefferis
Copy link
Collaborator

  • this still seems to be up in the air but at least some preparatory branches for igraph 2.0 use a 0 rather than NaN to signal unreachable nodes
  • see Compatibility with igraph 2.0.0 #517

Copy link

codecov bot commented Jan 16, 2024

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (c608307) 76.88% compared to head (ee4a57b) 76.89%.
Report is 2 commits behind head on master.

Files Patch % Lines
R/ngraph.R 85.71% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master     #518   +/-   ##
=======================================
  Coverage   76.88%   76.89%           
=======================================
  Files          48       48           
  Lines        5935     5937    +2     
=======================================
+ Hits         4563     4565    +2     
  Misses       1372     1372           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

verticestoprune=setdiff(seq_len(nvertices), verticestoprune)
} else {
verticestoprune=setdiff(verticestoprune, 0)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why this is necesary now.

Copy link
Collaborator Author

@jefferis jefferis Jan 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for the review @krlmlr, but this is still necessary because nat sets igraph::igraph_options(return.vs.es = FALSE) (because in the past I have found this makes a significant difference to some operations) and as noted here your recent changes to the handling of unreachable nodes are only valid when igraph::igraph_options(return.vs.es = TRUE). To me that's a bug, but I'm not sure if you plan to fix. Thanks! Greg.

* there are many functions deprecated in igraph v2.0.0
* unfortunately, I don't know what is the minimum igraph version for the new function names
  currently we only require igraph (>= 0.7.1)

get.vertex.attribute->vertex_attr
add.edges->add_edges
decompose.graph->decompose
graph.bfs->bfs
graph.dfs->dfs
is.connected->is_connected
is.directed->is_directed
get.vertex.attribute->vertex_attr
shortest.paths->distances
get.shortest.paths->shortest_paths

induced.subgraph -> induced_subgraph
no.clusters -> count_components
clusters -> components
minimum.spanning.tree -> mst
delete.vertices -> delete_vertices
set.vertex.attribute -> set_vertex_attr
get.edgelist -> as_edgelist
delete.edges -> delete_edges
* this still seems to be up in the air but at least some preparatory branches
 for igraph 2.0 use a 0 rather than NaN to signal unreachable nodes
* see #517
* used to be an int, now numeric with igraph 2.0
* I don't think this is so restrictive anymore and I think but have not fully checked that some of
  the functions deprecated in igraph 2.0 have been  replaced by functions defined in igraph 1.0
  but not earlier.
* there are still places in the code like

In file included from /home/runner/src/cmtk/core/libs/IO/cmtkImageFileDICOM.cxx:33:
/home/runner/src/cmtk/core/libs/IO/cmtkImageFileDICOM.h:45:10: fatal error: dcmtk/dcmdata/dctk.h: No such file or directory
   45 | #include <dcmtk/dcmdata/dctk.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~

where headers are expected
* otherwise it's not available
* there's a failure on github actions I can't yet figure out ...
@jefferis jefferis merged commit 3a6bf39 into master Oct 14, 2024
6 checks passed
@jefferis jefferis deleted the fix/igraph200 branch October 14, 2024 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants